-
Notifications
You must be signed in to change notification settings - Fork 164
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update mock-mount-s3 to require '--max-throughput-gbps' argument #1018
Update mock-mount-s3 to require '--max-throughput-gbps' argument #1018
Conversation
Signed-off-by: Daniel Carl Jones <djonesoa@amazon.com>
12f60c3
to
8464b47
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this binary really compile? I have to add features = ["thread-pool"]
for the futures crate to make it work. Maybe we should also fix that.
Signed-off-by: Daniel Carl Jones <djonesoa@amazon.com>
Yeah, I wasn't sure on the best practice here. I've submitted a fix in #1030. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good! thanks Danny.
Description of change
It was unclear to me when first using
mock-mount-s3
that it will limit throughput based on the value for--max-throughput-gbps
, and that this will default to 10Gbps as real Mountpoint does if it can't detect the correct value.This change requires the argument to be specified, so that the user can be forced to acknowledge this default or choose a better value for the use case.
Relevant issues: N/A
Does this change impact existing behavior?
It does, but only for the mock binary. You now must specify
--max-throughput-gbps <Gbps>
.Does this change need a changelog entry in any of the crates?
No.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and I agree to the terms of the Developer Certificate of Origin (DCO).